fix(files): reserve image layout space so images stop reflowing on load - #6299
Conversation
A markdown image with no stored dimensions reserved zero vertical space until it downloaded, then snapped to its natural height and pushed content below it down (cumulative layout shift). Reserve the box up front from the image's intrinsic aspect ratio instead. Store intrinsic width/height as workspace_file metadata (not in the markdown — it stays clean ``), read it synchronously from the already-loaded file list to reserve a responsive aspect-ratio box on first render, and lazily backfill it once per image on first view via a write-gated, idempotent PATCH. The node view falls back to on-load measurement for the first-ever view and for external images. Images stay fluid (max-width:100%, height:auto).
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
PR SummaryMedium Risk Overview A write-gated On the client, optional Reviewed by Cursor Bugbot for commit d2b1195. Configure here. |
Greptile SummaryThe PR adds persisted intrinsic image dimensions so markdown images can reserve responsive layout space before loading.
Confidence Score: 5/5The PR appears safe to merge. No blocking failure remains.
|
| Filename | Overview |
|---|---|
| apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/image.tsx | Reserves image space from stored or measured intrinsic dimensions and backfills mismatches after load. |
| apps/sim/hooks/queries/workspace-files.ts | Adds the cache-backed dimensions adapter, optimistic metadata updates, guarded persistence, and stale-write reconciliation. |
| apps/sim/app/api/workspaces/[id]/files/[fileId]/dimensions/route.ts | Adds an authenticated, write-gated endpoint that accurately reports content-version guard rejection. |
| apps/sim/lib/uploads/contexts/workspace/workspace-file-manager.ts | Maps and persists intrinsic dimensions while clearing stale values on content replacement. |
| packages/db/migrations/0282_chubby_psylocke.sql | Adds nullable width and height columns to workspace_files. |
Sequence Diagram
sequenceDiagram
participant UI as Markdown image
participant Cache as Workspace file cache
participant API as Dimensions API
participant DB as workspace_files
UI->>Cache: Read stored width and height
Cache-->>UI: Known dimensions or null
UI->>UI: Reserve aspect-ratio box
UI->>UI: Load image and measure natural size
UI->>Cache: Optimistically store measured dimensions
UI->>API: PATCH key, width, height
API->>DB: Update where workspace, file, and key match
DB-->>API: Written or rejected
API-->>UI: success true or false
alt Content key changed
UI->>Cache: Invalidate active file list
end
Reviews (10): Last reviewed commit: "docs(files): align stale dimension docs ..." | Re-trigger Greptile
…tent swap - onLoad guards on the memoized storedDimensions the render uses (not a fresh cache read), so a sibling's non-reactive backfill can't leave a view unreserved. - updateWorkspaceFileContent clears width/height when it swaps bytes, so stale dimensions can't be reserved for new content (and the null re-enables backfill). - Keep optimistically-cached dimensions when the PATCH fails (correct measurement; a 403/transient error shouldn't wipe sibling reservations). - Test imports the sibling via the absolute @/ path.
|
@cursor review |
…aring Clearing width/height to NULL on a content swap reopened the width IS NULL backfill path, so a late fire-and-forget PATCH for the previous image could write its stale size onto the new content. Instead, measure the new bytes' intrinsic dimensions server-side (image-size, headers only) and store those (or null for a non-image), so the row always matches the current content and a stale backfill can't apply.
|
@cursor review |
…rver-measuring Round-3 review: server-side image-size returns raw (non-EXIF) dimensions, and clearing dims on content swap reopened the stale-PATCH race for non-image or unmeasurable content. Move authority to the browser's own naturalWidth/Height (EXIF-correct): the node view reserves from it and reports on any mismatch, and updateWorkspaceFileDimensions overwrites (no width IS NULL gate) so stale values self-correct on the next view. Reverts the server-side measurement and the content-swap dimension touch entirely.
|
@cursor review |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit 17499b1. Configure here.
The self-heal rework left the old image's dimensions in the row after a content replacement, so the next view of the new bytes reserved a wrong-sized box before correcting. Clear width/height on the content-swap write so the row never describes stale content: the next view falls back to the baseline first-load reflow and the browser's measurement backfills the correct size. No server-side decode (EXIF-safe), and the client's overwrite-on-mismatch handles a late PATCH.
|
Addressed the remaining summary concern (stale metadata after content replacement): |
|
@cursor review |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit ba8507c. Configure here.
…n't persist Ties the dimensions write to the storage key the client measured. The key is regenerated on every content replacement, so an in-flight PATCH measured against superseded bytes is rejected at the DB (WHERE key = measured key) instead of persisting the old aspect ratio for new content. Closes the last stale-ordering window Greptile flagged — the write is now content-version-conditioned, not just corrected on the next render.
|
Closed the stale-ordering window at the write, as requested: |
|
@cursor review |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit cd1b2be. Configure here.
…p pass) Post-review /cleanup: the dimensions route TSDoc still described backfill-once behavior (now overwrite-on-mismatch via the content-key CAS); the bare-pixel width regex is hoisted to module scope. No behavior change.
|
@cursor review |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit 94348f0. Configure here.
…ns response
The route returned success:true even when updateWorkspaceFileDimensions matched
0 rows (the CAS rejected a write whose measured key no longer matches the row).
Return success:<whether a row was written> and widen the contract response to
{ success: boolean }. Not an error path — the client's next measurement persists
once its file list has the new key; this just stops the API claiming a persist
that did not happen.
|
@cursor review |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit 497b223. Configure here.
…sion-rejected Previously the client discarded a success:false (CAS-rejected) response, leaving its optimistic patch — which is for superseded bytes — lingering in the file-list cache. On rejection, invalidate the list so the cache reconciles with the new content (whose real size persists on its next load). Deliberately NOT a retry: re-sending the old measurement under the new key would write the wrong size. A transport error / read-only 403 still keeps the optimistic value (it's the real displayed size).
|
@cursor review |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit 4cabb83. Configure here.
…behavior Cleanup audit: the ImageDimensionsSource/reportImageDimensions interface docs and one route log string still said backfill-once/no-op; the mechanism overwrites on mismatch to self-correct. Wording only, no behavior change.
|
@cursor review |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit d2b1195. Configure here.
Summary
workspace_filemetadata (not in the markdown — it stays clean), read synchronously from the already-loaded file list to reserve a responsiveaspect-ratiobox on the first render, and lazily backfilled once per image on first view via a write-gated, idempotent PATCH.images. Images stay fluid (max-width:100%,height:auto).How it works
width/heightonworkspace_files(migration0282).updateWorkspaceFileDimensionsis idempotent (WHERE width IS NULL) and never bumpsupdatedAt(so it can't cache-bust the served image);PATCH …/[fileId]/dimensionsis write-gated.FileContentSourceseam (only the workspace source implements it); the adapter reads/patches the file-list cache;image.tsxreserves the box (memoized so a resize drag never re-scans the list) and measures + persists on load.Notes
Type of Change
Testing
/simplify+/cleanupover the diff.Checklist